Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some type safety for Fix #57314

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Feb 8, 2025

Disallows any type Fix{x} where !(x isa Int) || (x < 0).

Follows up on PR #54653.

The goals are similar as in the closed PR #56518, but less ambitious.

I hope it's not too late in the release cycle to consider this change as it seems not to be invasive or disruptive.

Disallows any type `Fix{x}` where `!(x isa Int) || (x < 0)`.

Follows up on PR JuliaLang#54653.

The goals are similar as in the closed PR JuliaLang#56518, but less ambitious.

I hope it's not too late in the release cycle to consider this change
as it seems not to be invasive or disruptive.
@nsajko
Copy link
Contributor Author

nsajko commented Feb 8, 2025

This PR leaves a kind of hole, in that negatives are disallowed by subtyping for the value of the type parameter, but zero is only disallowed by the constructor method. This could hypothetically be patched in a future PR by changing the offset from one-based to zero-based, however that seems like it'd be less worthwhile, so it makes sense to leave it for the future, if this one gets merged.

Even with the hole, this change is an improvement IMO, seeing as it makes expressions like Fix{Int} or Fix{0.0} throw.

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding random type parameters is slightly breaking and inefficient, so this is not going to get merged either unless you can show a compelling use for why this either prevents a soundness issue or enables a new feature

@nsajko nsajko closed this Feb 8, 2025
@nsajko nsajko deleted the partial_application_lightweight_type_safety branch February 8, 2025 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants